Add worker_class argument to LocalCluster #2625
Conversation
|
It looks like the failures are not related to this PR, but I'm not quite sure about it. |
| @@ -203,6 +206,8 @@ def __init__( | |||
| if security: | |||
| self.worker_kwargs["security"] = security | |||
|
|
|||
| self.worker_class = worker_class | |||
There was a problem hiding this comment.
| self.worker_class = worker_class | |
| if not worker_class: | |
| if processes: | |
| worker_class = Nanny | |
| else: | |
| worker_class = Worker | |
| self.worker_class = worker_class |
distributed/deploy/local.py
Outdated
| kwargs["quiet"] = True | ||
| else: | ||
| W = Worker | ||
| W = self.worker_class or Worker | ||
|
|
||
| w = yield W( |
There was a problem hiding this comment.
| w = yield W( | |
| w = yield self.worker_class( |
distributed/deploy/local.py
Outdated
| kwargs["quiet"] = True | ||
| else: | ||
| W = Worker | ||
| W = self.worker_class or Worker |
There was a problem hiding this comment.
If you accept the other changes, then we can probably drop the conditional above (it's effectively now handled in the constructor)
We might also remove the self.processes attribute, which seems to be effectively replaced and generalized by self.worker_class.
We should have something like the following though to maintain previous behavior.
if self.worker_class is Nanny:
kwargs['quiet'] = TrueThere was a problem hiding this comment.
Sure, your changes look fine to me. I will push a commit.
|
This could also use a test. Maybe something like ... def test_worker_class(loop):
class MyWorker(Worker):
pass
with LocalCluster(n_workers=2, loop=loop, worker_class=MyWorker, scheduler_port=0, dashboard_address=None) as cluster:
assert all(isinstance(w, MyWorker) for w in cluster.workers) |
|
Thanks for the suggestion, I was really unsure how to test this. I extended your test a bit to test both |
|
I should have mentioned that we recently switched to using |
Ah nice, didn't know about this, thanks for that! I think the errors are not related to this PR. |
|
Yeah, I'll handle them. Sorry for those :/
…On Fri, Apr 19, 2019 at 2:01 PM Peter Andreas Entschev < ***@***.***> wrote:
I should have mentioned that we recently switched to using black for code
styling. It looks like you found that out. I need to add docs for this in a
more prominent place, but you may want to try installing pre commit hooks
<https://ml.dask.org/contributing.html#pre-commit-hooks>
Ah nice, didn't know about this, thanks for that!
I think the errors are not related to this PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2625 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACKZTE4UTWGQOKFCBWTJSLPRIJIJANCNFSM4HHDP3JA>
.
|
|
No worries, let me know if you need help with them. |
Allows instantiating workers subclassed from Worker (e.g., CUDAWorker) in LocalCluster.
@mrocklin